fix(ui): polish marketplace announcement card and dedupe V1 card#28066
Conversation
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
V2 marketplace announcement card:
- Decouple text / background / border into three props on AnnouncementCardV1Content (backgroundColor, borderColor, currentBackgroundColor) so they can use distinct design tokens.
- Switch V2 to the blue-dark utility palette: --tw-color-utility-blue-dark-{50,500,600} for bg / border / text. Use the prefixed Tailwind variable name so it resolves at runtime.
- Compact variant now renders at 27px tall with rounded-4px corners, flex-none so a fixed width sticks, and px-10 / py-6 padding on the title-section only.
- Border moved from the inner .announcement-header to the outer .announcement-title-section so it follows the rounded corners.
V1 home page announcement card:
- Stop duplicating the JSX inlined during the Task-redesign migration (51ecf45) and delegate back to AnnouncementCardV1Content. The original Thread-based shape required fieldOperation/columnName which AnnouncementEntity no longer has - the migration inlined a stripped-down version instead of updating the shared component.
- Generalize AnnouncementCardV1Content so the rich header (user + icon + entity link) renders whenever userName or entityName is provided. field-operation-text stays gated on fieldOperation; title paragraph below renders whenever the rich header was used and title is set.
- V1 keeps its gradient look by passing the linear-gradient string through backgroundColor; V2 passes a solid color.
siddhant1
force-pushed
the
fix/marketplace-announcement-card-styling
branch
from
32e072a to
19498a7
Compare
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| const gradientBackground = currentBackgroundColor | ||
| ? `linear-gradient(270deg, #ffffff -12.07%, ${currentBackgroundColor} 500.72%)` | ||
| : undefined; |
There was a problem hiding this comment.
⚠️ Edge Case: V1 card gets solid background when currentBackgroundColor is falsy
When currentBackgroundColor is not provided (or falsy) to AnnouncementCardV1, gradientBackground will be undefined. In AnnouncementCardV1Content, this causes bgColor to fall back to color (which is PRIMARY_COLOR), resulting in a solid primary-color background + 3px border being applied.
The old inlined V1 code returned an empty style object {} when currentBackgroundColor was falsy — meaning no inline background/border at all. This is a behavioral regression for that edge case.
In practice, the parent AnnouncementsWidgetV1 always passes a bgColor derived from theme config, so this may not surface in production, but it's a latent bug if the component is reused or tested without that prop.
Always provide a gradient background (matching the old AnnouncementCardV1Content default behavior) so the fallback is consistent regardless of whether currentBackgroundColor is set.:
const gradientBackground = currentBackgroundColor
? `linear-gradient(270deg, #ffffff -12.07%, ${currentBackgroundColor} 500.72%)`
: `linear-gradient(270deg, #ffffff -12.07%, var(--ant-primary-color) 500.72%)`;
Was this helpful? React with 👍 / 👎
There was a problem hiding this comment.
Pull request overview
This PR refactors and polishes the marketplace announcement card UI by reusing the existing V1 announcement card content component, while adding support for separately configuring background and border colors so V2 can use distinct design tokens.
Changes:
- Added
backgroundColorandborderColoroverrides toAnnouncementCardV1Contentto decouple styling tokens. - Refactored V1 home-page announcement card to reuse
AnnouncementCardV1Contentinstead of duplicating JSX. - Updated V2 marketplace announcement item to use the shared content component with “blue-dark” palette tokens and compact variant styling.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| openmetadata-ui/src/main/resources/ui/src/components/MyData/Widgets/AnnouncementsWidgetV1/AnnouncementCardV1/AnnouncementCardV1Content.component.tsx | Adds color override props and updates compact/default variant styling and header/title rendering conditions. |
| openmetadata-ui/src/main/resources/ui/src/components/MyData/Widgets/AnnouncementsWidgetV1/AnnouncementCardV1/AnnouncementCardV1.component.tsx | Removes duplicated card markup and delegates rendering to AnnouncementCardV1Content. |
| openmetadata-ui/src/main/resources/ui/src/components/MyData/Widgets/AnnouncementsWidgetV1/AnnouncementCardV1/announcement-card-v1-content.less | Removes header border-left to support moving the accent border to the outer title section via inline styles. |
| openmetadata-ui/src/main/resources/ui/src/components/DataMarketplace/AnnouncementsWidgetV2/AnnouncementItemV2.component.tsx | Switches V2 to the shared content component and attempts to apply blue-dark tokenized styling. |
Comments suppressed due to low confidence (1)
openmetadata-ui/src/main/resources/ui/src/components/MyData/Widgets/AnnouncementsWidgetV1/AnnouncementCardV1/AnnouncementCardV1Content.component.tsx:60
- PR description mentions the compact variant should be 27px tall (and fixed width 247px), but the compact config still sets
tw:h-[30px]and does not set a fixed width. If the visual spec is still 27px/247px, please adjust the compact variant styles accordingly (or update the PR description to match the implemented sizing).
compact: {
header: 'tw:h-[30px] tw:flex-none tw:text-xs tw:rounded-[4px]',
titleSection: 'tw:px-[10px] tw:py-[6px] tw:pl-1',
entityName: 'tw:!text-[11px] tw:!font-normal',
iconSize: 'tw:size-[9px]',
title: 'tw:text-xs tw:font-medium tw:!mb-0',
| header: 'tw:h-[30px] tw:flex-none tw:text-xs tw:rounded-[4px]', | ||
| titleSection: 'tw:px-[10px] tw:py-[6px] tw:pl-1', |
| <AnnouncementCardV1Content | ||
| backgroundColor="var(--tw-color-utility-blue-dark-50)" | ||
| borderColor="var(--tw-color-utility-blue-dark-500)" | ||
| columnName={columnName} | ||
| currentBackgroundColor="var(--color-utility-blue-100)" | ||
| currentBackgroundColor="var(--tw-color-utility-blue-dark-600)" | ||
| description={description} |
siddhant1
added
UI
Jest test CoverageUI tests summary
|
🟡 Playwright Results — all passed (15 flaky)✅ 4055 passed · ❌ 0 failed · 🟡 15 flaky · ⏭️ 103 skipped
🟡 15 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
Update border and background tokens on AnnouncementsWidgetV2 to use utility-blue-dark-100 (#D1E0FF) / utility-blue-dark-50 (#EFF4FF) per the Untitled UI spec, align the loading skeleton with the loaded card, and drop the zero-padded count format (01 -> 1). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Switch the AnnouncementsWidgetV2 border from utility-blue-dark-100 to gray-blue-100 (#EAECF5) to match the updated design spec. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
openmetadata-ui/src/main/resources/ui/src/components/MyData/Widgets/AnnouncementsWidgetV1/AnnouncementCardV1/AnnouncementCardV1Content.component.tsx:231
- Behavior change for the V1 card: previously the title
<Typography.Paragraph className="announcement-title">was rendered unconditionally for every announcement. The shared component now renders the title paragraph only when(userName || entityName) && title. Although the fallback header branch showstitleas the header text whenuserName/entityNameare missing, an announcement that has both auserName/entityNameand atitlewill keep working, but a description-only announcement (no entity, no user) will lose the dedicated title block and instead show the title squeezed into the small header strip with the timestamp. Please confirm this is the intended behavior for the V1 home-page card or guard the title separately for V1.
{(userName || entityName) && title && (
<Typography.Paragraph
className={classNames('announcement-title', variantConfig.title)}
ellipsis={{ tooltip: true, rows: 2 }}>
{title}
{columnName && (
<Typography.Text>
{`${t('label.column-name')}: ${columnName}`}
</Typography.Text>
)}
</Typography.Paragraph>
)}
Code Review
|
| Compact |
|
Was this helpful? React with 👍 / 👎 | Gitar
|
Summary
AnnouncementCardV1Contentso V2 can use distinct design tokens. Switches V2 to theblue-darkutility palette (50 / 500 / 600 for bg / border / text), fixes the runtime CSS variable name (--tw-color-utility-blue-dark-*), and tightens the compact variant to 27px tall, fixed 247px wide, 4px corners, with the border on the outer title section.AnnouncementCardV1Content. Generalize the shared component so the rich header (user + icon + entity link) renders wheneveruserName/entityNameare provided —field-operation-textstays gated onfieldOperation. V1 keeps its gradient viabackgroundColor; V2 passes a solid color.Test plan
#EFF4FFbackground,#2970FFleft border,#155EEFtext, 27px height, 4px corners.AnnouncementCardV1.test.tsx— may need updates if it asserts on inlined DOM that's now delegated.yarn ui-checkstyle:changed.🤖 Generated with Claude Code